Android Autofill by LongCatIsLooong · Pull Request #17465 · flutter/engine

LongCatIsLooong · 2020-04-02T01:04:00Z

LongCatIsLooong · 2020-04-10T19:11:34Z

Hi @blasten could you take a look at this? I'm trying to add androidx.autofill to embedding_bundle, but not sure if I missed anything.

blasten · 2020-04-13T19:22:32Z

shell/platform/android/io/flutter/embedding/engine/systemchannels/TextInputChannel.java

We are very sensitive to adding dependencies to the embedding. This can cause conflict in the future if a plugin adds this dependency.
Since you are returning a String, is there any chance you can remove the dependency and return the raw string?

justinmc

LGTM. Take a look at my comment about fields, otherwise just nitpicks. 👍

justinmc · 2020-04-13T22:00:42Z

shell/platform/android/io/flutter/embedding/engine/systemchannels/TextInputChannel.java

Could this method be static?

justinmc · 2020-04-13T22:06:29Z

shell/platform/android/io/flutter/embedding/engine/systemchannels/TextInputChannel.java

There are two declarations of fields above, maybe some code got mixed up or missed being deleted.

blasten · 2020-04-14T03:42:59Z

shell/platform/android/io/flutter/embedding/engine/systemchannels/TextInputChannel.java

Suggested change

for (int i = 0; i < 16; i++) matrix[i] = jsonMatrix.getDouble(i);

for (int i = 0; i < 16; i++) {

matrix[i] = jsonMatrix.getDouble(i);

}

blasten · 2020-04-14T03:46:30Z

shell/platform/android/io/flutter/embedding/engine/systemchannels/TextInputChannel.java

nit: it might be a bit more readable if you do:

if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) { return hint; } // then switch

blasten · 2020-04-14T03:49:40Z

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

Suggested change

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O)

afm = view.getContext().getSystemService(AutofillManager.class);

else afm = null;

afm = Build.VERSION.SDK_INT >= Build.VERSION_CODES.O ? view.getContext().getSystemService(AutofillManager.class) : null;

SDK version check doesn't seem to work with ternary expressions, the linter started complaining when I applied the suggested change.

blasten · 2020-04-14T03:50:31Z

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

Suggested change

if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) return;

if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {

return;

}

blasten · 2020-04-14T03:50:48Z

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

Suggested change

if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) return;

if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {

return;

}

blasten · 2020-04-14T03:51:04Z

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

Suggested change

if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) return;

if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {

return;

}

blasten · 2020-04-14T03:57:10Z

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

I realized the code is missing braces in a few places. The Google style guide for Java suggests that braces are used even if there's only a single statement: https://google.github.io/styleguide/javaguide.html#s4.1.1-braces-always-used

Thank you for reviewing this!

All missing braces should be added now. Does the updated code look good to you?

blasten

LGTM

LongCatIsLooong added 8 commits March 26, 2020 17:01

wip

e6a7d23

wip

ff05c8d

update

056a393

WIP

34283bf

WIP

d3f71da

fix tests

4954bdf

add tests

2f31a97

indentation

0eaa510

auto-assign bot requested a review from iskakaushik April 2, 2020 01:04

googlebot added the cla: yes label Apr 2, 2020

LongCatIsLooong added 3 commits April 1, 2020 18:04

Merge branch 'master' into autofill

f121560

formatting

e87f57f

fix lint

a1967fd

LongCatIsLooong force-pushed the autofill branch from ab92aa4 to a1967fd Compare April 2, 2020 02:01

LongCatIsLooong mentioned this pull request Apr 3, 2020

Autofill Part 1 flutter/flutter#52126

Merged

13 tasks

LongCatIsLooong added 3 commits April 3, 2020 12:15

add "requestAutofill"

7b2ae2c

add androidx.autofill

086ab38

update DEPS

7197492

LongCatIsLooong force-pushed the autofill branch from d5d906a to 7197492 Compare April 10, 2020 19:02

rename allFields -> fields

e1c4334

blasten reviewed Apr 13, 2020

View reviewed changes

justinmc approved these changes Apr 13, 2020

View reviewed changes

LongCatIsLooong added 2 commits April 13, 2020 15:30

Merge remote-tracking branch 'upstream/master' into autofill

7423c08

Revert dependency changes

10a7a2b

LongCatIsLooong force-pushed the autofill branch from 404cffa to a1f3344 Compare April 13, 2020 23:57

review

480f843

LongCatIsLooong force-pushed the autofill branch from a1f3344 to 480f843 Compare April 14, 2020 00:22

blasten reviewed Apr 14, 2020

View reviewed changes

review

0ca379f

LongCatIsLooong force-pushed the autofill branch from 435ac06 to 0ca379f Compare April 14, 2020 04:55

blasten approved these changes Apr 15, 2020

View reviewed changes

LongCatIsLooong merged commit cf3d738 into flutter:master Apr 16, 2020

LongCatIsLooong deleted the autofill branch April 16, 2020 10:41

engine-flutter-autoroll mentioned this pull request Apr 16, 2020

Roll engine e6a2534b63ac..f4d6ce13dcc4 (32 commits) flutter/flutter#54997

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 16, 2020

cf3d738 Android text input autofill (flutter/engine#17465)

b7380a7

bparrishMines mentioned this pull request Mar 29, 2023

[webview_flutter] Autofill doesn't work flutter/flutter#92165

Closed

-                for (int i = 0; i < 16; i++) matrix[i] = jsonMatrix.getDouble(i);
+                for (int i = 0; i < 16; i++) {
+                  matrix[i] = jsonMatrix.getDouble(i);
+                }

-    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O)
-      afm = view.getContext().getSystemService(AutofillManager.class);
-    else afm = null;
+      afm = Build.VERSION.SDK_INT >= Build.VERSION_CODES.O ? view.getContext().getSystemService(AutofillManager.class) : null;

-    if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) return;
+    if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {
+      return;
+    }

Conversation

LongCatIsLooong commented Apr 2, 2020

Uh oh!

LongCatIsLooong commented Apr 10, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blasten left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments